Fix Windows SFTP open flags and wolfsshd -D argument parsing(f8827) - #1173
Fix Windows SFTP open flags and wolfsshd -D argument parsing(f8827)#1173miyazakh wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request fixes Windows-specific behavior in wolfSSH’s SFTP server open handling and wolfsshd argument parsing, and adds Windows regression coverage to prevent regressions in the SFTP open-flag matrix.
Changes:
- Correct Windows SFTP
RecvOpencreation-disposition handling by mappingCREAT/EXCL/TRUNCto a single validCreateFile()disposition and wiringAPPENDaccess. - Fix Windows
wolfsshd -Ddetection by comparingCommandLineToArgvW()wide arguments withwcscmp(L"-D"). - Extend internal SFTP test plumbing to build under
USE_WINDOWS_APIand add a Windows open-flag matrix regression test.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| wolfssh/wolfsftp.h | Enables SFTP internal test hooks on Windows (except POSIX-fd invalidation helper). |
| src/wolfsftp.c | Adds SFTP_WinCreationDisp() and fixes Windows RecvOpen access/disposition handling; adjusts internal test hook gating for Windows. |
| tests/regress.c | Refactors shared SFTP reply assertion helper to build on Windows and adds TestSftpWindowsOpenFlagMatrix(). |
| apps/wolfsshd/wolfsshd.c | Fixes -D parsing on Windows by using wcscmp() with wide string literals. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #1173
Scan targets checked: wolfssh-bugs, wolfssh-src
Findings: 6
6 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Findings are non-blocking.
ejohnstown
left a comment
There was a problem hiding this comment.
I sent comments directly.
Summary
wolfSSH_SFTP_RecvOpen()'s Windows (USE_WINDOWS_API) path builtdwCreationDispositionby OR-ing togetherOPEN_EXISTING/CREATE_ALWAYSbits, butCreateFile()'s creation-disposition parameter is a single enumerated value, not a bitmask. TRUNC and EXCL were also never wired up (left under#if 0), and APPEND access was missing. AddSFTP_WinCreationDisp()to resolve the SFTP CREAT/EXCL/TRUNC flag combination to the one correctCreateFile()disposition, and OR inFILE_APPEND_DATAforWOLFSSH_FXF_APPEND.wolfsshd's-D(foreground/no-daemon) argument check on Windows comparedcmdArgs[i]withWSTRCMP, butcmdArgsentries come fromCommandLineToArgvWand are wide strings. Comparing them as narrowchar*data meant-Dwas never recognized. Usewcscmp()againstL"-D"instead.WOLFSSH_TEST_INTERNALregression-test plumbing inwolfsftp.c/wolfsftp.hto build underUSE_WINDOWS_APItoo (it was previously guarded out on Windows), except forwolfSSH_SFTP_TestInvalidateHeadFd(), which stays POSIX-only since it manipulates a raw fd and Windows tracks aHANDLEinstead.TestSftpWindowsOpenFlagMatrix()(tests/regress.c), which walks theRecvOpenCREAT/EXCL/TRUNC flag matrix on Windows and checks both the open result and the resulting file state for each case:WRITEonly, noCREAT, missing file -> fails, file not createdWRITE|CREAT, missing file -> creates it (OPEN_ALWAYS)WRITE|CREAT, existing file -> opens without truncatingWRITE|CREAT|TRUNC, existing file -> truncates immediatelyWRITE|CREAT|EXCL, existing file -> failsWRITE|CREAT|EXCL, missing file -> succeedsREAD|WRITE|CREAT, missing file -> creates itTesting
Built and ran the full test suite on Windows via MSYS2 MinGW64 (
_WIN32->USE_WINDOWS_API):tests/regress.test.exeincludes the newTestSftpWindowsOpenFlagMatrix(), exercising the correctedCreateFile()disposition logic end to end.scripts/external.testandscripts/fwd.testare skipped on Windows as expected (external network / Unix-only port forwarding).